Implement Invalidate search type for Potential Match rules (CO-2690) #70
Conversation
There was a problem hiding this comment.
Should this overall capability be configurable? Partially for backwards compatibility, but also because not every deployment might want potential matches to drop to no matches.
This could perhaps most easily be done by splitting the "Invalidate" search type into (eg) "Invalidate Canonical" and "Invalidate All". "Invalidate Canonical" would basically be the current search type relabeled, and would effectively be a no-op if set on a Potential Rule (probably the UI shouldn't permit that, see also CO-2181). "Invalidate All" might not be the best label.
| $results = $potentialMatches->getRawResults(); | ||
|
|
||
| // Check if any Attribute Rules in "Invalidate" mode will demote this result. | ||
| foreach($results as $rowId => $attrs) { |
There was a problem hiding this comment.
This loop is fairly duplicative of the canonical check, and perhaps should be refactored into a separate call.
| } | ||
|
|
||
| // No need to continue with any loop | ||
| break 2; |
There was a problem hiding this comment.
Is 2 correct here? (It's 3 in the canonical loop.)
There was a problem hiding this comment.
I think this should be "break 2", not "break 3" because we need to continue processing any other potential matches to determine if they also should be invalidated.
Once we have successfully dropped a potential match to no match, there's no need to continue with the loop for rule attributes or for rules. But we need to continue with the loop that iterates through all of the $results.
| $found = $potentialMatches->remove($rowId); | ||
| if (!$found) { | ||
| Log::write('warning', $sor . "/" . $sorid . " Invalidate mode - error occurred removing result from potential match result set ". $rowId . "\t" . $results[$rowId]['sorid']); | ||
| } |
There was a problem hiding this comment.
Under what circumstances is this error likely to happen and be actionable by an administrator?
There was a problem hiding this comment.
This feels like one of those "this line should never be printed" messages. This would only occur if we tried to remove() a potential match from the result set because it passes the invalidate test, but remove() can't find the rowId in the result set. It seems unlikely to occur.
I could make this scenario fatal and throw a RuntimeException in case it does happen.
Currently, the "Invalidate" search type has no effect when applied to a Potential match rule. So, I wasn't particularly concerned about breaking compatibility, as no existing Potential match rules should (intentionally) be configured with "Invalidate" that would be affected by this change. |
Extend support for the Invalidate search type to Potential Match rules. Drop a Potential Match to No Match when the Invalidate attribute does not match.